Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement paymaster #582

Merged
merged 26 commits into from
Mar 14, 2024
Merged

feat: implement paymaster #582

merged 26 commits into from
Mar 14, 2024

Conversation

yutianwu
Copy link
Collaborator

@yutianwu yutianwu commented Mar 1, 2024

Description

This pr will implements BEP bnb-chain/BEPs#362.

Rationale

Implement paymaster.

Example

n/a

Changes

Notable changes:

  • add MsgSetBucketFlowRateLimit

Potential Impacts

  • add new message
  • affect all the fee-charging related messages of storage module, like CreatObject, CreateBucket and etc.

app/upgrade.go Outdated Show resolved Hide resolved
@@ -110,3 +110,12 @@ message LocalVirtualGroup {
// Notice that the minimum unit of charge is 128K
uint64 total_charge_size = 4;
}

message BucketFlowRateLimit {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BucketFlowRateLimit is just Int. Why we need this instead of using Int?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add another fields

x/storage/keeper/grpc_query.go Outdated Show resolved Hide resolved
x/storage/keeper/payment.go Outdated Show resolved Hide resolved
x/storage/keeper/keeper_rate_limit.go Outdated Show resolved Hide resolved
k.deleteBucketFlowRateLimitForDifferentBucketOwner(ctx, paymentAccount, bucketOwner, bucketName)

// set the flow rate limit for the bucket for the current bucket owner
err := k.setFlowRateLimit(ctx, bucket, paymentAccount, bucketName, rateLimit)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setFlowRateLimit the function name seem not good, it is simliar to setBucketFlowRateLimit, but it does a lot of things.

}

internalBucketInfo := k.MustGetInternalBucketInfo(ctx, bucketInfo.Id)
isRateLimited := k.isBucketRateLimited(ctx, bucketName)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is very hard to understand isRateLimited from the word.. any better wording?

if currentFlows != nil {
flowList = append(flowList, *currentFlows)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the motivation to change the code here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the previous code don't allow empty currentFlows and previousFlows, but when we resume the bucket, the previousFlows is empty

@yutianwu yutianwu added this pull request to the merge queue Mar 14, 2024
Merged via the queue into bnb-chain:develop with commit e4cc584 Mar 14, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants